Add tool calling to the LLM base class, implement in OpenAI #322
Conversation
oskarhane
force-pushed
the
feature/llm-tool-invocation
branch
from
cd47dc2 to
882915f
Compare
oskarhane
force-pushed
the
feature/llm-tool-invocation
branch
from
fc2f734 to
7f9bb39
Compare
oskarhane
force-pushed
the
feature/llm-tool-invocation
branch
from
913c8be to
b769242
Compare
oskarhane
force-pushed
the
feature/llm-tool-invocation
branch
2 times, most recently
from
59bbb6a to
9ae73cb
Compare
To not rely on json schema from openai
oskarhane
force-pushed
the
feature/llm-tool-invocation
branch
from
107d670 to
61a0e46
Compare
| minimum: Optional[int] = None | ||
| maximum: Optional[int] = None | ||
|
|
||
| def model_dump_tool(self) -> Dict[str, Any]: |
There was a problem hiding this comment.
Is model_dump_tool only used to discard null values? If so, there is an option in model_dump for this (exclude_none=True by memory)
There was a problem hiding this comment.
I know I'm a bit nitpicking here, but I've started to work on the implementation for Vertex AI, and some parameters need to be excluded, so if we can rely on Pydantic for this instead of manually re-implementing it, that would be great :) but if it's not possible, it's not possible.
There was a problem hiding this comment.
It does a bit more than that mapping some names "min_items" -> "minItems", but that could be removed by naming the parameters "minItems" directly.
Your call, do you want me to change it?
There was a problem hiding this comment.
Ok I see, no do not change anything now, we'll add improvements as we need them. They are unit tested so that's great :)
| raise ValueError("Parameter type is required") | ||
|
|
||
| # Find the appropriate class based on the type | ||
| param_classes = { |
There was a problem hiding this comment.
I feel like we could get rid of many of these by using either a field discriminator or some union, but I won't be strict on this point, experiments can be left for a cool down period.
There was a problem hiding this comment.
Sounds good, let's look into it later
Description
Type of Change
Complexity
Complexity: Medium
How Has This Been Tested?
Checklist
The following requirements should have been met (depending on the changes in the branch):